Skip to content

Conversation

@reubeno
Copy link
Owner

@reubeno reubeno commented Mar 11, 2025

Rebases #200 against latest changes in tree + adapts to move platform-specific code under sys. Also adds a couple of basic compat tests to cover the functionality.

Huge credit to @39555 for sorting out the problem and getting this working! I've created this under a separate PR to avoid stomping on the previous branch.

@reubeno reubeno requested a review from Copilot March 11, 2025 02:37
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR implements the command –p functionality, rebasing against recent changes and moving platform‐specific code under the sys module. Key changes include:

  • Updating command behavior to support the –p flag by modifying both the builtins and command execution logic.
  • Adjusting the shell’s PATH resolution and default executable search paths.
  • Adding new compatibility tests and updating platform-specific filesystem utilities.

Reviewed Changes

File Description
brush-core/src/builtins/command.rs Modified command lookup and execution to support use_default_path; updated try_find_command signature.
brush-core/src/shell.rs Updated PATH initialization and refactored executable search functions to use default paths.
brush-core/src/sys/unix/fs.rs Added constants and functions for executable and standard utils path retrieval, including confstr integration.
brush-shell/tests/cases/builtins/command.yaml Added new test cases for command -p functionality and command -v -p usage.
brush-core/src/commands.rs Modified command execution to accept an optional path_dirs parameter for enhanced external command lookup.
brush-core/src/sys/stubs/fs.rs Stub implementations updated for default path functions.
brush-core/src/interp.rs Adapted function calls to new execute signature with path_dirs parameter.

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

@github-actions
Copy link

github-actions bot commented Mar 11, 2025

Test Results

    2 files      9 suites   1m 35s ⏱️
  694 tests   694 ✅ 0 💤 0 ❌
1 374 runs  1 374 ✅ 0 💤 0 ❌

Results for commit f7891b2.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Mar 11, 2025

Performance Benchmark Report

Benchmark name Baseline (μs) Test/PR (μs) Delta (μs) Delta %
clone_shell_object 18.52 μs 18.10 μs -0.42 μs 🟢 -2.25%
eval_arithmetic 0.17 μs 0.17 μs 0.00 μs ⚪ Unchanged
expand_one_string 1.79 μs 1.91 μs 0.13 μs 🟠 +7.11%
for_loop 22.18 μs 22.71 μs 0.52 μs 🟠 +2.36%
function_call 2.36 μs 2.31 μs -0.05 μs ⚪ Unchanged
instantiate_shell 57.72 μs 57.38 μs -0.34 μs ⚪ Unchanged
instantiate_shell_with_init_scripts 24172.57 μs 23891.37 μs -281.20 μs ⚪ Unchanged
parse_bash_completion 1674.30 μs 1665.56 μs -8.74 μs ⚪ Unchanged
parse_sample_script 1.76 μs 1.77 μs 0.01 μs ⚪ Unchanged
run_echo_builtin_command 14.90 μs 15.25 μs 0.35 μs ⚪ Unchanged
run_one_external_command 2082.11 μs 2078.34 μs -3.78 μs ⚪ Unchanged
tokenize_sample_script 2.88 μs 2.78 μs -0.10 μs 🟢 -3.33%

Code Coverage Report: Only Changed Files listed

Package Base Coverage New Coverage Difference
brush-core/src/builtins/command.rs 🟢 91.01% 🟢 94.12% 🟢 3.11%
brush-core/src/commands.rs 🟢 86.97% 🟢 87.24% 🟢 0.27%
brush-core/src/interp.rs 🟢 91.53% 🟢 91.54% 🟢 0.01%
brush-core/src/shell.rs 🟢 82.54% 🟢 83.03% 🟢 0.49%
brush-core/src/sys/unix/fs.rs 🟠 56.41% 🟠 65.96% 🟢 9.55%
Overall Coverage 🟢 76.03% 🟢 76.1% 🟢 0.07%

Minimum allowed coverage is 70%, this run produced 76.1%

Test Summary: bash-completion test suite

Outcome Count Percentage
✅ Pass 1338 63.71
❗️ Error 231 11.00
❌ Fail 160 7.62
⏩ Skip 357 17.00
❎ Expected Fail 13 0.62
✔️ Unexpected Pass 1 0.05
📊 Total 2100 100.00

39555 and others added 2 commits March 10, 2025 22:55
Adjusts previous changes to address earlier PR feedback; adds tests.
@reubeno reubeno force-pushed the command-p-revisited branch from 62bd2e7 to f7891b2 Compare March 11, 2025 05:55
@reubeno reubeno merged commit a96ad90 into main Mar 11, 2025
18 checks passed
@reubeno reubeno deleted the command-p-revisited branch March 11, 2025 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants